After some discussion, I think this PR needs some more time before it can be merged, and unfortunately won't be part of the next release.

The current implementation is removing all thousands separators other than what is specified as the "decimal" separator, which is quite risky and may leads to problems. It's better to follow what pandas is doing, i.e., have both decimal and thousands as separators. By default, the thousands separator should be None (so no replacement).

If there is some kind of weird string like 1,2.3,4, it should not be parsed as a number. I am not sure how far we should do to parse something like 1,2.34 with decimal . and thousands ,: it's not a format I recognize, but it would still be recognized as 12.34 rather than being rejected.

Another check that may be considered is counting the number of decimal separators, and reject any case where there is more than one.

Some additional comments:

• While it's impossible to test all possible scenarios, tests should also include as many weird edge cases as we can come up with to see what could be the result.
• The ToFloat docstring needs some more work to explain in more detail the behavior when decimal and thousands are set.

I'll convert this back to draft and keep an eye on this for the next PR.

When talking about the tests, it was mentioned to include 3 tests:

• A test for Good inputs
• A test for Bad Inputs
• A test for bad parameters
  I merged the last two tests including also bada parameters in the test. If it's better to have the 3 tests individually instead of the 2, I will modify it.

Thanks a lot for the PR @gabrielapgomezji! This will be very useful for parsing data that is not in the usual locale.

My comments are mostly about improving clarity in the documentation and adding comments in the code. I think the actual content of the PR is in a good shape, it's just a matter of polishing at this point.

I did a very quick and dirty benchmark comparing the performance of the ToFloat transformer in the latest version of skrub, and the ToFloat in this PR.

I generated a dataframe with 10M rows and 30 columns.

There is a small, but noticeable difference in time when there is no conversion to be done, so we might want to add a condition where the formatting check is skipped in the default case (decimal="." and thousands=None).

skrub version: 0.8.dev0
Elapsed time for ToFloat transformation: 0.3026 seconds
skrub version: 0.7.1
Elapsed time for ToFloat transformation: 0.2914 seconds

import time
import polars as pl
import numpy as np
import skrub
from importlib import metadata
import polars.selectors as cs

version = metadata.version("skrub")
print(f"skrub version: {version}")

# Set random seed for reproducibility
np.random.seed(42)

# Generate random float data
data = np.random.uniform(1000, 1000000, size=(10_000_000, 30))

# Convert floats to strings with space as thousands separator
df = pl.DataFrame(data)

from skrub import ToFloat, ApplyToCols
def benchmark_tofloat(df):
    tic = time.time()
    transformer = ApplyToCols(ToFloat())
    transformed_df = transformer.fit_transform(df)
    toc = time.time()

    return toc - tic

# Run benchmark
times = []
for run in range(100):
    elapsed_time = benchmark_tofloat(df)
    times.append(elapsed_time)

elapsed_time = np.median(times)
print(f"Elapsed time for ToFloat transformation: {elapsed_time:.4f} seconds")